FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management#2341
FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management#2341asmasarw wants to merge 3 commits intoredhat-developer:mainfrom
Conversation
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
d7564fa to
17df515
Compare
| credentials: dangerously-allow-unauthenticated | ||
| resourceOptimization: | ||
| costManagement: | ||
| clientId: ${ROS_CLIENT_ID} |
There was a problem hiding this comment.
@asmasarw if it is called costManagement, why do we still have env variables with ROS_ shouldn't it be refactored too?
There was a problem hiding this comment.
agree,
changed to CM_
| const baseUrl = | ||
| configApi.getOptionalString('costManagementProxyBaseUrl') ?? | ||
| configApi.getOptionalString( | ||
| 'costManagement.costManagementProxyBaseUrl', |
There was a problem hiding this comment.
@asmasarw this seem to be a breaking change, you added a prefix.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
| 'costManagement.costManagementProxyBaseUrl', | ||
| ) ?? | ||
| configApi.getOptionalString( | ||
| 'costManagement.optimizationsBaseUrl', |
There was a problem hiding this comment.
@asmasarw here too, the prefix was added not refactored.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
| @@ -15,7 +15,7 @@ | |||
| */ | |||
|
|
|||
| export interface Config { | |||
| resourceOptimization: { | |||
| costManagement: { | |||
There was a problem hiding this comment.
As I commented before, previously this was used without prefix, and after the refactor you introduced a prefix, is thsi expect? is this the config?
There was a problem hiding this comment.
Here the name is fine, but the nested values were incorrect.
Reverted.
| * Override generated pluginId so discovery matches the backend plugin ID | ||
| * (OpenAPI info.title is set to 'cost-management' for the API name). | ||
| */ | ||
| setPluginIdValue(pluginIdFilePath, value) { |
There was a problem hiding this comment.
Why do we need this? If this is a refactoring PR, why do we have new code. I don't understand why is this relevant and previously wasnt.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
| console.log('Setting pluginId to backend plugin ID for discovery'); | ||
| tsSourceFileMutator.setPluginIdValue( | ||
| `${commonPackageDir}/src/generated/pluginId.ts`, | ||
| 'redhat-resource-optimization', |
There was a problem hiding this comment.
This was done by mistake, Reverted.
| @@ -120,7 +120,7 @@ export const getLegendData = ( | |||
|
|
|||
| const result: any = []; | |||
|
|
|||
| series.map((s, index) => { | |||
| series.forEach((s, index) => { | |||
There was a problem hiding this comment.
This is a Sonar Qube Issue fixed here.
CI fail here, unless we're changing the map to forEach.
2c6e89d to
fec97ca
Compare
| @@ -2,23 +2,23 @@ | |||
| "packageRules": [ | |||
| { | |||
| "description": "all Redhat Resource Optimization minor updates", | |||
There was a problem hiding this comment.
Update this to Red hat Cost Management instead of Resource Optimization
alizard0
left a comment
There was a problem hiding this comment.
Please review the .github/renovate-presets/workspace/rhdh-redhat-resource-optimization-presets.json file. I found multiple entries for Resource Optimization and I think you are renaming to Cost Management no?
| "matchFileNames": ["workspaces/redhat-resource-optimization/**"], | ||
| "matchFileNames": ["workspaces/cost-management/**"], | ||
| "extends": [ | ||
| "github>redhat-developer/rhdh-plugins//.github/renovate-presets/base/rhdh-patch-presets(RH Resource Optimization)" |
There was a problem hiding this comment.
Same applies here, if i am not wrong. Cost Management is the new name for it, right?
There was a problem hiding this comment.
Renamed, But It fails on:
Renovate Config Validator - as I understand it's because it tries to search for the preset in the upstream, but since we are renaming this, it doesn't exist yet in the Upstream branch.
Once merged this CI validation will pass.
PreetiW
left a comment
There was a problem hiding this comment.
Incomplete Plugin Rename — redhat-resource-optimization → cost-management
The workspace directory was renamed to cost-management/, but the plugins, packages, IDs, routes, configs, and scripts all still use redhat-resource-optimization. This needs to be completed for the dynamic plugin image to build and install correctly under the new name.
What's done ✅
- Workspace directory:
workspaces/cost-management/ repository.directoryfields in allpackage.jsonfiles- README title uses "Cost Management"
- OpenAPI spec title uses
cost-management - Parent menu item in
dynamic-plugins.yamlusescost-management
What's still using the old name ❌
1. Plugin directories
plugins/redhat-resource-optimization/→plugins/cost-management/plugins/redhat-resource-optimization-backend/→plugins/cost-management-backend/plugins/redhat-resource-optimization-common/→plugins/cost-management-common/
2. NPM package names (all package.json)
@internal/redhat-resource-optimization→@internal/cost-management@red-hat-developer-hub/plugin-redhat-resource-optimization→@red-hat-developer-hub/plugin-cost-management@red-hat-developer-hub/plugin-redhat-resource-optimization-backend→@red-hat-developer-hub/plugin-cost-management-backend@red-hat-developer-hub/plugin-redhat-resource-optimization-common→@red-hat-developer-hub/plugin-cost-management-common- All
backstage.pluginId,backstage.pluginPackages, and workspace dependencies need updating
3. Plugin IDs in source
plugins/.../src/plugin.ts(FE):id: 'redhat-resource-optimization'plugins/.../src/plugin.ts(BE):pluginId: 'redhat-resource-optimization'plugins/.../src/generated/pluginId.ts:pluginId = 'redhat-resource-optimization'plugins/.../src/apis.ts:id: 'plugin.redhat-resource-optimization.api'
4. Route paths
update the route paths as well to /cost-management
5. Dynamic plugin config
dynamic-plugins.yaml: package path, plugin key, menu item keys (redhat-resource-optimization,redhat-resource-optimization.ocp)app-config.dynamic.yaml(FE + BE): plugin keys- Note:
importNamevalues (ResourceOptimizationPage,OpenShiftPage,ResourceOptimizationIcon) should stay as-is — they are code export names describing the feature.
6. Config & metadata
app-config.yaml:app.title: Red Hat - Resource Optimizationcatalog-info.yaml:metadata.nameandmetadata.titlevs.code-workspace:window.title
7. scripts/dynamic-plugins.sh
update the reference and regenerate the image, check if image gets generated correctly and used correctly
8. Common lib internals
scripts/lib/openapi.mjs:spec.info.titlescripts/generate_client.mjs:DEFAULT_PLUGIN_DIRECTORYsrc/clients/optimizations/types.ts: comment reference
9. Root package.json scripts
start:fe-pluginandstart:be-pluginworkspace references
10. Documentation
- Plugin READMEs: npm package names, route paths, endpoint references
docs/dynamic-plugin.md: any old name references
Post-rename tasks
- Regenerate
yarn.lock(yarn install) - Regenerate API reports (
yarn build:api-reports) - Test dynamic plugin build (
scripts/dynamic-plugins.sh oci) - Update
.github/renovate-presets/workspace/rhdh-redhat-resource-optimization-presets.json
@asmasarw once you update the above let me know and I will test and review again 🙌🏻
| { | ||
| "description": "all Redhat Resource Optimization dev dependency updates", | ||
| "matchFileNames": ["workspaces/redhat-resource-optimization/**"], | ||
| "matchFileNames": ["workspaces/cost-management/**"], |
There was a problem hiding this comment.
@asmasarw can we rename this file as well please to avoid confusion?
|



FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management